Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TEP-0090: Matrix - Minimal Status is Required #5019

Merged
merged 1 commit into from
Jun 22, 2022

Conversation

jerop
Copy link
Member

@jerop jerop commented Jun 22, 2022

Changes

TEP-0090: Matrix proposed executing a PipelineTask in
parallel TaskRuns and Runs with substitutions from combinations
of Parameters in a Matrix.

The status of PipelineRuns with fanned-out PipelineTasks will
list all the TaskRuns and Runs created.

In TEP-0100 we proposed changes to PipelineRun status
to reduce the amount of information stored about the status of
TaskRuns and Runs to improve performance, reduce memory bloat
and improve extensibility. With the minimal PipelineRun status,
we can handle Matrix without exacerbating the performance and
storage issues that were there before.

In this change, we validate that minimal embedded status is enabled
when a PipelineTask has a Matrix.

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

  • Docs included if any changes are user facing
  • Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including functionality, content, code)
  • Release notes block below has been filled in (if there are no user facing changes, use release note "NONE")

Release Notes

The `embedded-status` feature flag must be set to `"minimal"` to specify `Matrix` in a `PipelineTask`.

@tekton-robot tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 22, 2022
@abayer
Copy link
Contributor

abayer commented Jun 22, 2022

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 22, 2022
@jerop
Copy link
Member Author

jerop commented Jun 22, 2022

Related PR: tektoncd/community#736

@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1beta1/pipeline_types.go 96.8% 96.8% 0.0
pkg/apis/pipeline/v1beta1/status_validation.go Do not exist 100.0%

[TEP-0090: Matrix][tep-0090] proposed executing a `PipelineTask` in
parallel `TaskRuns` and `Runs` with substitutions from combinations
of `Parameters` in a `Matrix`.

The status of `PipelineRuns` with fanned-out `PipelineTasks` will
list all the `TaskRuns` and `Runs` created.

In [TEP-0100][tep-0100] we proposed changes to `PipelineRun` status
to reduce the amount of information stored about the status of
`TaskRuns` and `Runs` to improve performance, reduce memory bloat
and improve extensibility. With the minimal `PipelineRun` status,
we can handle `Matrix` without exacerbating the performance and
storage issues that were there before.

In this change, we validate that minimal embedded status is enabled
when a `PipelineTask` has a `Matrix`.

[tep-0090]: https://github.com/tektoncd/community/blob/main/teps/0090-matrix.md
[tep-0100]: https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md
@jerop jerop force-pushed the tep-0090-minimal-required branch from d6c142d to 5d388a1 Compare June 22, 2022 18:00
@tekton-robot tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 22, 2022
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1beta1/pipeline_types.go 96.8% 96.8% 0.0
pkg/apis/pipeline/v1beta1/status_validation.go Do not exist 100.0%

@abayer
Copy link
Contributor

abayer commented Jun 22, 2022

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 22, 2022
@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dibyom

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 22, 2022
@tekton-robot tekton-robot merged commit 1755579 into tektoncd:main Jun 22, 2022
@jerop
Copy link
Member Author

jerop commented Jun 23, 2022

/kind feature

@tekton-robot tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 23, 2022
jerop added a commit to jerop/pipeline that referenced this pull request Jun 24, 2022
In tektoncd/community#736, we decided that
`Matrix` will use `ChildReferences` only. Therefore, it is unnecessary
to check the `TaskRuns` field of `PipelineRun` status when getting the
names of `TaskRuns` for matrixed `PipelineTasks`.

Related PR: tektoncd#5019
jerop added a commit to jerop/pipeline that referenced this pull request Jun 24, 2022
In tektoncd/community#736, we decided that
`Matrix` will use `ChildReferences` only. Therefore, it is unnecessary
to check the `TaskRuns` field of `PipelineRun` status when getting the
names of `TaskRuns` for matrixed `PipelineTasks`.

Related PR: tektoncd#5019
tekton-robot pushed a commit that referenced this pull request Jun 24, 2022
In tektoncd/community#736, we decided that
`Matrix` will use `ChildReferences` only. Therefore, it is unnecessary
to check the `TaskRuns` field of `PipelineRun` status when getting the
names of `TaskRuns` for matrixed `PipelineTasks`.

Related PR: #5019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants